Fix explanation of script tag change #6996
Conversation
The description says we added "to new tags" which probably was supposed to be "two new tags". But even with that it's a bit confusing because there is one tag added compared to the previous step, that is the tag for `angular-route.js`. I just updated the doc to say this more clearly.
|
Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.
If you need to make changes to your pull request, you can update the commit with Thanks again for your help! |
| @@ -129,10 +129,13 @@ __`app/index.html`:__ | |||
| </html> | |||
| ``` | |||
|
|
|||
| We have added to extra `<script>` tags in our index file to load up extra JavaScript files into our | |||
| We have added an extra `<script>` tag to our index file to load up an extra JavaScript file into our | |||
There was a problem hiding this comment.
This should be:
We have added two extra
<script>tags in our index file to load up extra JavaScript files into our
application:
angular-route.js: defines thengRoutemodule.controllers.js: defines a newphonecatControllersmodule.
There was a problem hiding this comment.
it could be that too, but there is really one script tag added compared to the previous step... but that'd be better than "to" too. I can change if you want.
There was a problem hiding this comment.
No, there are two script tags added, however controllers.js is present in step 6. The one which seems to be added is app.js.
I think maybe the tutorial app is broken, because that doesn't seem right. @petebacondarwin tutorial guru, does it actually work? I don't see how the injector gets created properly at all until step 7, that seems broken.
|
I'll sort this out. It may be that the tutorial content is still not quite in synch with angular-phonecat, which definitely works |
|
I agree that it works, but it's not clear how it works --- it's only loading a script which defines the phonecatControllers module, but bootstraps the app with phonecatApp. That doesn't make any sense! |
|
Oh, apparently I'm wrong. There is no phonecatControllers app anymore -- or at least for the early steps in the tutorial, the controllers file creates phonecatApp. That makes sense, then. |
|
Oh, I see. We add the app.js since we are now creating a top level app that "depends" upon the controllers. I'll reword the tutorial step. |
|
I'm sorry, but I wasn't able to verify your Contributor License Agreement (CLA) signature. CLA signature is required for any code contributions to AngularJS. Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match. If you signed the CLA as a corporation, please let us know the company's name. Thanks a bunch! PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR. |
|
CLA signature verified! Thank you! Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes). |
|
Many thanks :)
|
Request Type: docs
How to reproduce:
Component(s): ngRoute
Impact: small
Complexity: small
This issue is related to:
Detailed Description:
Other Comments:
The description says we added "to new tags" which probably was supposed to be "two new tags". But even with that it's a bit confusing because there is one tag added compared to the previous step, that is the tag for
angular-route.js. I just updated the doc to say this more clearly.